-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Use compiler default include paths #515
feat: Use compiler default include paths #515
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very exciting. As soon as this is merged I'll remove all this logic from the script that generates the boost.url docs: https://github.com/alandefreitas/url/blob/eccc2b85bfecc0d897dca049409cf1b0ddfa2485/doc/generate-files.mjs#L116-L147
I forgot to mention, but the logic in these lines should be removed so that the tests ensure mrdocs is finding the appropriate include paths and to avoid including these paths twice. |
Done! removed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great.
Now that I'm looking at Compiler.h/cpp
, this looks kind of weird because it sounds like there are functions able to compile code. Maybe CompilerInfo.h/cpp
? I'm just throwing ideas. I'm not sure. It's up to you.
I'm also a little worried about MSVC compiler identification, although this might work anyway as long as there's no "#include <...> search starts here:"
in the MSVC output, which is kind of a way of identifying MSVC anyway.
These are just ideas. If you think it's good to go, then it's good to go.
I removed the MSVC detection (using filename) because it will fail when called with the arguments I also renamed |
Great! So this is good to go, right? |
Yes
…On Tue, Dec 19, 2023, 6:30 PM Alan de Freitas ***@***.***> wrote:
Great! So this is good to go, right?
—
Reply to this email directly, view it on GitHub
<#515 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACC2EYEKUP4CHNVCMEUUE3YKHFMJAVCNFSM6AAAAABATDO7HKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRTGIYDGOJQGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Have you checked the output in the CI artifacts? I don't want to merge two things in a row with problems in the output 😆 |
Please squash before you merge! |
@alandefreitas @sdkrystian |
I checked the output of the "Demos" job in Github Actions and everything seems fine https://github.com/cppalliance/mrdocs/actions/runs/7262807028/job/19787446683?pr=515#logs |
No description provided.